Claude finished @Hmbown's task in 5m 19s —— View job

PR Review: Route inline reasoning streams

• Gather context and understand request
• Read diff and changed files
• Review correctness and logic
• Check provider/model/route safety
• Check reuse and simplification
• Check test coverage
• Post findings

Summary

The routing logic is sound and the state machine for split-chunk tag parsing is correct. A few issues worth addressing before this ships, in priority order:

1. Silent fallback on unrecognized config value — parse_reasoning_stream_style (chat.rs:2165–2173)

fn parse_reasoning_stream_style(value: &str) -> Option<ReasoningStreamStyle> {
    match value.trim().to_ascii_lowercase().replace('-', "_").as_str() {
        ...
        _ => None,
    }
}

reasoning_stream_style_for_stream calls this and, on None, falls through to the provider/model heuristic. A user who typos "inline_tag" (singular) gets silently ignored — their configured gateway behaviour is then driven by is_reasoning_model_for_stream, which returns SeparateField for reasoning providers and None otherwise. Neither is what they intended. A logging::warn (or even a debug log) when the configured value goes unrecognized would surface this during setup. Fix this →

2. InlineTags mode also consumes reasoning_content fields (chat.rs:2742–2751)

if reasoning_stream_style != ReasoningStreamStyle::None
    && let Some(reasoning) = reasoning_text.as_deref()
{
    push_thinking_delta(...);
}

The condition != None means InlineTags also routes a reasoning_content/reasoning delta to push_thinking_delta. For a gateway configured with inline_tags that also happens to emit reasoning_content (e.g. an OpenRouter passthrough with metadata), both paths activate: reasoning_content starts a Thinking block, then the content string with <think> tags is split by inline_reasoning_segments. The block-transition logic in push_text_delta/push_thinking_delta handles it correctly (no panic, no dropped content), but the downstream consumer sees more thinking-block starts than expected.

If the intent is "InlineTags gateways don't use the separate field", the condition should be == ReasoningStreamStyle::SeparateField. If the intent is "route both if present", that's fine but should be documented. Fix this →

3. Missing test: unclosed <think> tag at stream end (chat.rs:2891–2915)

The flush path (inline_reasoning_segments("", state, true) at finish_reason) correctly emits remaining pending content as Thinking when inside_think == true. But there is no test for this — a truncated stream ending with an open <think> and no matching </think>. Worth a dedicated #[test] that sends a single chunk with <think> content and no close tag, followed by a finish-reason chunk, asserting the partial reasoning is emitted rather than silently dropped. Fix this →

4. Module-level doc comment names a now-test-only function (chat.rs:4)

//! ... SSE parsing (`parse_sse_chunk`) all live here.

parse_sse_chunk is now #[cfg(test)]; the production entry point is parse_sse_chunk_with_reasoning_style. The doc comment should be updated to avoid confusion. Fix this →

False alarm from Gemini

The inline comment suggestion on config/tests.rs:4241 (use std::time::SystemTime::now() fully qualified) is not needed — SystemTime and UNIX_EPOCH are already imported at config/tests.rs:8 via use std::time::{SystemTime, UNIX_EPOCH}.

What's good

• trailing_tag_prefix_len — the is_char_boundary guard handles UTF-8 content (e.g. CJK text before a tag) correctly; the O(tag.len()) scan is fine for 7–9 char tags.
• push_reasoning_segment empty filter — prevents spurious ContentBlockStart events when a split produces an empty leading/trailing segment.
• #[cfg(test)] shim on parse_sse_chunk — clean pattern; the old signature is a thin wrapper around parse_sse_chunk_with_reasoning_style and gated so it doesn't pollute production exports. All callers in client.rs are inside #[cfg(test)] mod tests, so nothing breaks.
• Config merge at merge_provider_config — the new field is included correctly with or semantics (override wins, then base).
• reasoning_stream_style_for_stream override priority — explicit config beats model/provider heuristic, which is the right default for OpenAI-compatible gateways.
• Block transition logic in push_text_delta/push_thinking_delta — correctly stops the opposite block and increments content_index before opening the new one; no index bookkeeping bugs found.

Claude finished @Hmbown's task in 2m 41s —— View job

PR Review: Route inline reasoning streams (updated)

• Gather context and understand request
• Check what changed in latest commits vs previous review
• Read updated diff and key files
• Review correctness and logic
• Check if previous findings were addressed
• Post findings

Follow-up on Previous Findings

The two commits pushed since the initial review (7993d15 and 6900776) fully address all four issues that were flagged. Quick verification:

Fresh Review of Current State

The implementation looks clean. A few observations on the final shape:

State machine (inline_reasoning_segments) — the trailing_tag_prefix_len hold-back loop is correct, including the is_char_boundary guard for multi-byte content. The flush = true path correctly zero-holds and emits everything, so truncated streams don't silently drop content.

Match structure in parse_sse_chunk_with_reasoning_style — the four arms are exhaustive and non-overlapping:

• (Some, InlineTags) → segment through inline_reasoning_segments
• (Some, _) → plain text (covers SeparateField and None with a content field)
• (None, None) → fall through reasoning_text as text (legacy passthrough gateways)
• (None, _) → no-op (reasoning field already handled or not applicable)

This correctly preserves the legacy behavior for unknown gateways while adding the two new explicit modes.

parse_sse_chunk test shim — gated #[cfg(test)], creates a fresh InlineReasoningTagState per call (correct for the pre-existing tests that don't test inline tag cross-chunk state). New tests use decode_chunks_with_style which correctly carries state across chunks.

Config — reasoning_stream_style stored as Option<String> on DeepSeekClient and parsed lazily at stream start; merge_provider_config uses or semantics (override wins). Both are correct.

Test coverage — 7 new tests cover: separate field routing, inline tag routing (including split across chunks), unclosed-tag flush, ignoring reasoning_content in InlineTags mode, None passthrough, override priority, and config deserialization.

Minor notes (not blocking):

• parse_reasoning_stream_style accepts several undocumented aliases ("inline", "think_tags", "separate", "text", "off", etc.). The doc only mentions the canonical three. These won't cause user confusion, but if the aliases are intentional they're worth a note in CONFIGURATION.md.
• The tag match is case-sensitive (<think> only). That's the de-facto standard for these gateways, so it's fine for the current scope.

No new correctness issues found. This is ready to merge.